Reorganise file layout and install for multiple platforms. #26

ltratt · 2023-12-29T09:24:53Z

This commit has two aims. First, install new files (systemd, bash completion) as appropriate: this forces us to drop BSD make compatibility (oh well). Second, regoranise the repository a bit to allow future expansion for other non-Linux platforms, which may also want to include extra files like these.

@hseg Does this still work as you'd expect on Linux?

This commit has two aims. First, install new files (systemd, bash completion) as appropriate: this forces us to drop BSD make compatibility (oh well). Second, regoranise the repository a bit to allow future expansion for other non-Linux platforms, which may also want to include extra files like these.

hseg · 2023-12-30T20:39:16Z

It works! Though I am unsure this needs to be gated behind a test for Linux -- the systemd files shouldn't cause trouble on other systems, and might serve as useful documentation there. If you insist, perhaps use a USE_SYSTEMD flag?
BTW, reviewing the Makefile, I notice a couple of nonstandard oddities:

No .PHONY rule, so if ever there are files named install, test or distrib in the project root, make might get confused
Usage of $MAN_PREFIX (entirely nonstandard) and $PREFIX to control installation location, instead of the more usual respective $(DESTDIR)$(MANDIR) and $(DESTDIR)$(PREFIX)
Why are all the files read-only? Just judging by permissions on the executable, this puts pizauth in a club only shared on my system by bashbug and db's tools. While there is no need for this permission, neither is there a need to lock it down.

ltratt · 2024-01-01T08:26:15Z

Though I am unsure this needs to be gated behind a test for Linux

Potentially other OSs will also introduce similar files -- I don't think it's useful to install stuff that's not relevant and that might thus confuse people. That said, if systemd starts to become used on non-Linuxes, this might be worth revisiting.

No .PHONY rule, so if ever there are files named install, test or distrib in the project root, make might get confused

Good point! Fixed in 61aff49.

Usage of $MAN_PREFIX (entirely nonstandard) and $PREFIX to control installation location, instead of the more usual respective $(DESTDIR)$(MANDIR) and $(DESTDIR)$(PREFIX)

Also a good point! I hadn't realised the difference. Fixed in e6f54fe.

Why are all the files read-only? Just judging by permissions on the executable, this puts pizauth in a club only shared on my system by bashbug and db's tools. While there is no need for this permission, neither is there a need to lock it down.

Good question. Partly just force of habit I think. But, in general, it's hard to go wrong with fewer permissions, so I'll leave that as-is unless doing so causes concrete problems.

If those two new commits still work correctly for you (they seem to for me) then I think we'll be good to cut a new release!

hseg · 2024-01-01T21:28:37Z

Looking at e6f54fe, I must've been unclear -- the idea is to prefix all the installation target locations with $(DESTDIR) (so that the installation tree can be constructed in a temporary directory for packaging) -- $(MANDIR), $(PREFIX) and the like are still independently valuable. cf the following snippets from the yt-dlp Makefile (arbitrarily selected off my machine):

PREFIX ?= /usr/local
BINDIR ?= $(PREFIX)/bin
MANDIR ?= $(PREFIX)/man
SHAREDIR ?= $(PREFIX)/share
PYTHON ?= /usr/bin/env python3

# set SYSCONFDIR to /etc if PREFIX=/usr or PREFIX=/usr/local
SYSCONFDIR = $(shell if [ $(PREFIX) = /usr -o $(PREFIX) = /usr/local ]; then echo /etc; else echo $(PREFIX)/etc; fi)

install: lazy-extractors yt-dlp yt-dlp.1 completions
	mkdir -p $(DESTDIR)$(BINDIR)
	install -m755 yt-dlp $(DESTDIR)$(BINDIR)/yt-dlp
	mkdir -p $(DESTDIR)$(MANDIR)/man1
	install -m644 yt-dlp.1 $(DESTDIR)$(MANDIR)/man1/yt-dlp.1
	mkdir -p $(DESTDIR)$(SHAREDIR)/bash-completion/completions
	install -m644 completions/bash/yt-dlp $(DESTDIR)$(SHAREDIR)/bash-completion/completions/yt-dlp
	mkdir -p $(DESTDIR)$(SHAREDIR)/zsh/site-functions
	install -m644 completions/zsh/_yt-dlp $(DESTDIR)$(SHAREDIR)/zsh/site-functions/_yt-dlp
	mkdir -p $(DESTDIR)$(SHAREDIR)/fish/vendor_completions.d
	install -m644 completions/fish/yt-dlp.fish $(DESTDIR)$(SHAREDIR)/fish/vendor_completions.d/yt-dlp.fish

I'm not sure which of these variables BINDIR, MANDIR, SHAREDIR are standard, but the DESTDIR/PREFIX distinction (and coexistence) is

(Needless to say, my build system complains that all of this is installing outside of /usr)

ltratt · 2024-01-01T23:20:09Z

You weren't unclear -- I was stupid. See if f1ecf8d makes more sense.

hseg

Package linter's only remaining complaint is that my invocation of make as make PREFIX="/usr" DESTDIR="$pkgdir/" install places the manpages at /usr/man where my distro expects /usr/share/man. Have a few remaining minor remarks, but otherwise LGTM

hseg · 2024-01-02T23:06:56Z

Makefile

@@ -1,20 +1,35 @@
 PREFIX ?= /usr/local
-MAN_PREFIX ?= ${PREFIX}/man
+BINDIR ?= $(PREFIX)/bin
+MANDIR ?= $(PREFIX)/man


man hier on my system indicates man pages should be installed either to /usr/local/man or /usr/share/man, with /usr/man being deprecated in favor of /usr/share/man.
Looking around for prior art on this, I notice that autotools uses (haven't read the rest of the script, so I'm not sure why they're single-quoting here)

bindir='${exec_prefix}/bin' sbindir='${exec_prefix}/sbin' libexecdir='${exec_prefix}/libexec' datarootdir='${prefix}/share' datadir='${datarootdir}' sysconfdir='${prefix}/etc' sharedstatedir='${prefix}/com' localstatedir='${prefix}/var' runstatedir='${localstatedir}/run' includedir='${prefix}/include' oldincludedir='/usr/include' docdir='${datarootdir}/doc/${PACKAGE_TARNAME}' infodir='${datarootdir}/info' htmldir='${docdir}' dvidir='${docdir}' pdfdir='${docdir}' psdir='${docdir}' libdir='${exec_prefix}/lib' localedir='${datarootdir}/locale' mandir='${datarootdir}/man'

man hier on my system indicates man pages should be installed either to /usr/local/man or /usr/share/man, with /usr/man being deprecated in favor of /usr/share/man.

The only way I can see to handle this is some sort of conditional in the Makefile, unless I'm missing out on something.

One hack I've seen here to avoid conditionals comes from batsignal:

MANPREFIX.$(PREFIX)=$(PREFIX)/share/man MANPREFIX./usr/local=/usr/local/man MANPREFIX.=/usr/share/man MANPREFIX=$(MANPREFIX.$(PREFIX))

That is very clever! I guess in our context this would become:

MANDIR.$(PREFIX) = $(PREFIX)/share/man MANDIR./usr/local = /usr/local/man MANDIR. = /usr/share/man MANDIR ?= $(MANDIR.$(PREFIX))

?

hseg · 2024-01-02T23:07:27Z

Makefile

-	install -d ${PREFIX}/share/examples/pizauth
-	install -c -m 444 pizauth.conf.example ${PREFIX}/share/examples/pizauth
+	install -d ${DESTDIR}${BINDIR}
+	install -c -m 555 target/release/pizauth ${DESTDIR}${PREFIX}/bin/pizauth


Minor remark: could've reused ${BINDIR} instead of spelling it out again

Fixed in abf06e0.

hseg · 2024-01-02T23:13:50Z

Makefile

+	install -d ${DESTDIR}${SHAREDIR}/pizauth/bash
+	install -c -m 444 share/bash/completion.bash ${DESTDIR}${SHAREDIR}/pizauth/bash/completion.bash
+ifeq ($(PLATFORM), Linux)
+	install -d ${DESTDIR}${PREFIX}/lib/systemd/user


cf autotools remark above, could introduce a $LIBDIR variable for this

hseg · 2024-01-02T23:14:03Z

Makefile

+	install -d ${DESTDIR}${MANDIR}/man5
+	install -c -m 444 pizauth.1 ${DESTDIR}${MANDIR}/man1/pizauth.1
+	install -c -m 444 pizauth.conf.5 ${DESTDIR}${MANDIR}/man5/pizauth.conf.5
+	install -d ${DESTDIR}${SHAREDIR}/examples/pizauth


Might be a BSDism, but on my system pizauth is the only package installing examples to /usr/share/examples -- everyone else uses /usr/share/$pkgname (optionally with an examples subdirectory`). Can patch this in my packaging if you're attached to this layout.

ltratt · 2024-01-03T08:45:25Z

f093aec introduces LIBDIR (as suggested) and EXAMPLESDIR. Setting the latter means that packagers can fairly easily cope with a BSD (default) or Linux layout. My gut feeling is that doing anything more than this will probably add quite a lot of complexity, although I might be wrong about that!

hseg · 2024-01-03T21:08:17Z

f093aec introduces LIBDIR (as suggested) and EXAMPLESDIR. Setting the latter means that packagers can fairly easily cope with a BSD (default) or Linux layout. My gut feeling is that doing anything more than this will probably add quite a lot of complexity, although I might be wrong about that!

Already this much makes packaging to Linux standards easier, thank you!

hseg · 2024-01-06T17:20:42Z

On Thu, Jan 04, 2024 at 02:42:01AM -0800, Laurence Tratt wrote: @ltratt commented on this pull request. > @@ -1,20 +1,35 @@ PREFIX ?= /usr/local -MAN_PREFIX ?= ${PREFIX}/man +BINDIR ?= $(PREFIX)/bin +MANDIR ?= $(PREFIX)/man That is very clever! I guess in our context this would become: ``` MANDIR.$(PREFIX) = $(PREFIX)/share/man MANDIR./usr/local = /usr/local/man MANDIR. = /usr/share/man MANDIR ?= $(MANDIR.$(PREFIX)) ``` ?

I _think_ it should work, but haven't tested it on my machine. I do know that `batsignal`, on my machine, with `PREFIX=/usr`, indeed installs its manpages to `/usr/share/man`, but I would recommend testing this just to make sure.

hseg · 2024-01-06T17:56:44Z

Makefile

-	install -d ${PREFIX}/share/examples/pizauth
-	install -c -m 444 pizauth.conf.example ${PREFIX}/share/examples/pizauth
+	install -d ${DESTDIR}${BINDIR}
+	install -c -m 555 target/release/pizauth ${DESTDIR}${BINDIR}/pizauth


Til this is a BSDism -- both the separation of -d away from the installation of files, and the use of -c make little sense under linux, where -D (combining install -d $dir and install $file $dir) exists and -c is ignored.
https://man.archlinux.org/man/install.1
https://man.freebsd.org/cgi/man.cgi?query=install&sektion=1

Nothing to change, just a TIL

Also use a cunning trick for `MANDIR` (based on an example pointed out to me in batsignal) to select a location that works as expected for both BSD and Linux file systems.

ltratt · 2024-01-07T09:06:16Z

@hseg Thanks for your help on this one!

Add PHONY targets.

61aff49

ltratt force-pushed the reorganise_install branch from e6f54fe to f1ecf8d Compare January 1, 2024 23:19

hseg suggested changes Jan 2, 2024

View reviewed changes

hseg reviewed Jan 6, 2024

View reviewed changes

Use DESTDIR as the means for controlling installation.

c79bed0

Also use a cunning trick for `MANDIR` (based on an example pointed out to me in batsignal) to select a location that works as expected for both BSD and Linux file systems.

ltratt force-pushed the reorganise_install branch from f093aec to c79bed0 Compare January 7, 2024 08:32

ltratt merged commit 1af7401 into master Jan 7, 2024
5 checks passed

ltratt deleted the reorganise_install branch January 7, 2024 09:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reorganise file layout and install for multiple platforms. #26

Reorganise file layout and install for multiple platforms. #26

ltratt commented Dec 29, 2023

hseg commented Dec 30, 2023

ltratt commented Jan 1, 2024

hseg commented Jan 1, 2024 •

edited

Loading

ltratt commented Jan 1, 2024

hseg left a comment

hseg Jan 2, 2024

ltratt Jan 2, 2024

hseg Jan 3, 2024

ltratt Jan 4, 2024

hseg Jan 2, 2024

ltratt Jan 2, 2024

hseg Jan 2, 2024

hseg Jan 2, 2024

ltratt commented Jan 3, 2024

hseg commented Jan 3, 2024

hseg commented Jan 6, 2024 via email

hseg Jan 6, 2024

ltratt commented Jan 7, 2024

Reorganise file layout and install for multiple platforms. #26

Reorganise file layout and install for multiple platforms. #26

Conversation

ltratt commented Dec 29, 2023

hseg commented Dec 30, 2023

ltratt commented Jan 1, 2024

hseg commented Jan 1, 2024 • edited Loading

ltratt commented Jan 1, 2024

hseg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ltratt commented Jan 3, 2024

hseg commented Jan 3, 2024

hseg commented Jan 6, 2024 via email

Choose a reason for hiding this comment

ltratt commented Jan 7, 2024

hseg commented Jan 1, 2024 •

edited

Loading